Skip to content

Conversation

@redindelible
Copy link
Contributor

No description provided.

Copy link
Contributor

@mpkarpov-ui mpkarpov-ui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the changes overall, just a couple questions though

Comment on lines +101 to +102
;lib_deps =
; Eigen
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why eigen is killed here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should undo this getting deleted

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guessing these are just organizational / formatting changes so lgtm

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RIP yessir.cpp :(

Comment on lines +24 to +41
enum class LED {
BLUE = 0,
RED = 1,
ORANGE = 2,
GREEN = 3
};

/**
* The ESP32 has two physical cores, which will each be dedicated to one group of threads.
* The DATA_CORE runs the GPS thread, as well as the threads which read from the sensor_data struct (e.g. SD logging).
*/
#define DATA_CORE ((BaseType_t) 1)
enum class Channel {
A = 0,
B = 1,
C = 2,
D = 3
};

/**
* Macro for declaring a thread. Creates a function with the name suffixed with `_thread`, annotated with [[noreturn]].
*
* @param name The name of the task.
* @param param The single parameter to input into the thread.
*/
#define DECLARE_THREAD(name, param) [[noreturn]] void name##_thread(param)
enum class Camera {
Side = 0,
Bulkhead = 1
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we're switching to enum classes instead of defines here?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better than the #DEFINE pattern

Comment on lines -109 to +80
float roll_rate_hz = std::clamp(std::abs(orientation.angular_velocity.vx) / (2.0f*static_cast<float>(PI)), 0.0f, max_roll_rate_hz);
float roll_rate_hz = std::clamp(std::abs(orientation.angular_velocity.vx) / (2.0f*static_cast<float>(M_PI)), 0.0f, max_roll_rate_hz);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought PI and M_PI had different values?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm just misremembering

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought M_PI was something like ~ 0.27, might be mistaken though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants